-
Notifications
You must be signed in to change notification settings - Fork 749
Add logical and op #13342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add logical and op #13342
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13342
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 2 Pending, 1 Unrelated FailureAs of commit 002e655 with merge base f7df6b1 ( NEW FAILURE - The following job has failed:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
This PR needs a
|
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
69ec0bb to
84e712a
Compare
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
84e712a to
1c4b1e4
Compare
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
1c4b1e4 to
5fa2334
Compare
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
5fa2334 to
e442791
Compare
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
|
Can I get a review on this PR? @haowhsu-quic @shewu-quic @winskuo-quic @DannyYuyang-quic |
e442791 to
06e4b08
Compare
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
DannyYuyang-quic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for supporting more ops!!
| @dataclass(init=False, frozen=True) | ||
| class OpElementWiseAnd: | ||
| op_name: str = "ElementWiseAnd" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @cccclai, since we already have OpElementWiseAnd at line 111, I believe there's no need to add it again. Thanks!!
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
06e4b08 to
23535ff
Compare
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
|
|
||
| @register_node_visitor | ||
| class And(NodeVisitor): | ||
| target = ["aten.logical_and.default"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could extend the target field in builders/op_and.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to do that, however I found out the logical_and op doesn't have the corresponding q/dq ops when trying module = self.get_qdq_module(module, sample_input), do you know which path I should be updating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to extend annotation with new target in quantizer/annotator.py
| @register_annotator([torch.ops.aten.__and__.Tensor]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the latest version correct? It seems still missing the q/dq ops
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is because the input is bool type so it won't have qdq. Could you try either 1) FP input. 2) When calling get_qdq_module in UT, also set bypass_check=True?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I believe the reason is because the input is bool (uint8) rather than float.
| if _is_float_tensor(input_act0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thanks! Yeah it's indeed the reason
23535ff to
26aac45
Compare
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
26aac45 to
895c92a
Compare
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
|
This pull request was exported from Phabricator. Differential Revision: D80122607 |
Summary: As title, add the logical and op for an internal model Differential Revision: D80122607
895c92a to
873609d
Compare
Summary: As title, add the logical and op for an internal model Differential Revision: D80122607
873609d to
002e655
Compare
|
I have it fix, does the new version look good now? @shewu-quic @haowhsu-quic @winskuo-quic @DannyYuyang-quic |
|
LGTM. Thanks! |
Summary: As title, add the logical and op for an internal model Rollback Plan: Differential Revision: D80122607
Summary:
As title, add the logical and op for an internal model
Rollback Plan:
Differential Revision: D80122607